fix: refget endpoint reported an incorrect Content-Length, leading to RuntimeErrors - #847
Conversation
The refget sequence endpoint set Content-Length to the length of the whole sequence on every response, including subsequence requests. A request for NC_000016.10?start=2077608&end=2085800 streamed 8192 bytes under a declared 90338345, and the ASGI server aborted the response with "Response content shorter than Content-Length" once the status line was already committed. GZipMiddleware strips Content-Length from compressed responses, so only clients requesting identity encoding were affected, and TestClient's default gzip request meant no existing test could observe the header. - Resolve start/end to concrete half-open bounds as soon as the sequence length is known, and derive validation, Content-Length, Content-Range, and the generator bounds from those same values - Run bounds validation whenever either bound is supplied, not only when both are; a one-sided out-of-range request previously skipped validation and returned an empty 200 carrying the full sequence length - Apply the same one-sided bounds check to the seqrepo sequence endpoint, which never set Content-Length and so could not crash, but returned a truncated body under a 200 for coordinates past the end of the sequence. It now rejects them with 422, the code that endpoint already used for start > end - Cover every request shape with tests asserting Content-Length equals the returned body length, pinning Accept-Encoding: identity so gzip cannot mask a regression Closes #846
…erage Tighten the marker definitions so the distinction is about mocking and scope rather than a vague notion of size, and mark the refget and seqrepo router suites accordingly. - Redefine `integration` as an end-to-end multi-component flow with no internal mocking, and `unit` as fast and isolated, explicitly allowing real collaborators when they are local, fast, and deterministic - Mark tests/routers/test_refget.py and tests/routers/test_seqrepo.py as unit suites - Cover the refget service-info endpoint, including the HGVS_SEQREPO_DIR-derived data version and its "unknown" fallback - Cover the 400 returned when start/end query parameters are combined with a Range header
Coverage Report for CI Build 32058748293Warning No base build found for commit Coverage: 89.066%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
|
Claude did a more thorough job reviewing this change than me, so I'll quote its report: Code Review: mavedb-api #847PR: fix: refget endpoint reported an incorrect SummaryThe core fix is correct: Note: the PR's own description already discloses that Findings1. Malformed Range survives every guard —
|
3 participants
Summary
The refget sequence endpoint declared
Content-Lengthas the full length of the sequence on every response, including subsequence requests.NC_000016.10?start=2077608&end=2085800streamed 8,192 bytes under a declared 90,338,345, and the ASGI server aborted the response withRuntimeError: Response content shorter than Content-Lengthafter the status line was already committed. Clients got a truncated body.Both sequence endpoints now resolve
start/endto concrete half-open bounds once the sequence length is known, and derive validation, headers, and the streamed body from those same values.Changes
Content-Lengthreports the bytes actually streamed. Bounds validation runs whenever either bound is supplied, not only when both are. A one-sided out-of-range request previously skipped validation entirely and returned an empty 200 carrying the full sequence length.Content-Lengthand so could not crash, but returned a truncated body under a 200 for coordinates past the end of a sequence.Content-Length == len(body); added coverage for service-info and the range/query-param conflict; sharpened theunit/integrationmarker definitions.Behavior changes
Requests that were broken and now work:
RuntimeErrorserver-sideContent-Length?start=1,?end=3)Content-LengthRequests that were silently wrong and are now rejected:
Content-RangestartNote
GZipMiddlewarestripsContent-Lengthfrom compressed responses, andTestClientrequests gzip by default. That is why this reached production with the endpoint under test, and why the new assertions pinAccept-Encoding: identity. Without that header they pass regardless of the fix.